Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure GOV.UK Frontend component selectors cannot conflict when initialised #1443

Merged
merged 11 commits into from
Jun 13, 2019

Conversation

NickColley
Copy link
Contributor

@NickColley NickColley commented Jun 11, 2019

This change introduces to main changes:

  1. govuk- prefix to all [data-module] selectors to avoid conflicts.
    We have found users using the same name without a prefix which results in a conflicts that break components: Namespacing [data-module] to avoid conflicts #1218

  2. Initialising the button and details component with a [data-module] selector so all components are initialised consistently.

Doing this for the button is important as it unblocks a bugfix for the 'prevent double click' feature: #1433

It will allow us to fix this issue without adding JavaScript behaviour to all buttons on pages, which could result in performance or conflict issues.

Closes #1218

This ensures that the components cannot accidentally conflict with other components with the same name.
… selector

We want to make sure we're only adding JavaScript enhancements to buttons that clearly have the `[data-module="govuk-button"]` attribute.

This will allow us to fix an issue where we want to be able to scope the 'prevent double click' feature from failing on multiple buttons.
…e selector

We want to make sure we're only adding JavaScript enhancements to details elements that clearly have the `[data-module="govuk-details"]` attribute.

This makes sure the GOV.UK Frontend is future friendly and makes this component consistent with the rest of the components.
These are now consistent with all components
@NickColley NickColley changed the title Namespace data modules [WIP] Namespace data modules Jun 11, 2019
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1443 June 11, 2019 11:08 Inactive
@NickColley NickColley added this to the v3.0.0 milestone Jun 11, 2019
@NickColley NickColley changed the title [WIP] Namespace data modules [WIP] Namespace [data module] selector with govuk- and initialise button and details components with [data-module] selectors Jun 11, 2019
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a changelog entry

@NickColley NickColley changed the title [WIP] Namespace [data module] selector with govuk- and initialise button and details components with [data-module] selectors Ensure GOV.UK Frontend component selectors cannot conflict when initialised Jun 12, 2019
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1443 June 12, 2019 10:25 Inactive
var $buttons = document.querySelector('[data-module="govuk-buttons"]')
```

### Details component
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to avoid the duplication here? I duplicated it to be really clear but it makes it lengthy

@@ -268,6 +268,77 @@

([PR #1376](https://github.com/alphagov/govuk-frontend/pull/1376))

- Ensure GOV.UK Frontend component selectors cannot conflict when initialised
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m-green would be good to get your thoughts on this

CHANGELOG.md Outdated

If you're using Nunjucks macros, and are [using the `initAll` function](https://github.com/alphagov/govuk-frontend/blob/master/docs/installation/installing-with-npm.md#option-1-include-javascript) you will not be affected.

If you are using HTML, you will need to add the `govuk-` prefix to any `[data-module]` attributes referencing GOV.UK Frontend components.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if

referencing GOV.UK Frontend components.

is clear? Could we say

any [data-module] attributes on GOV.UK Frontend component HTML

We could add

and any [data-module] attributes referencing GOV.UK Frontend scripts in your custom components

although it seems like a fairly unlikely scenario.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or alternatively

any [data-module] attributes in HTML which reference GOV.UK Frontend Javascript
🤷‍♀

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will try and make this clearer thanks Hanna :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved the example around and adjusted the wording so it hopefully makes it clearer, lemme know what you think.

CHANGELOG.md Outdated

If you are using HTML, you will need to add the `govuk-` prefix to any `[data-module]` attributes referencing GOV.UK Frontend components.

If you are initialising components manually, you will need to add the `govuk-` prefix to any selectors referencing GOV.UK Frontend components:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any Javascript selectors

to make it super clear.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1443 June 12, 2019 15:59 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1443 June 12, 2019 16:15 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1443 June 13, 2019 10:27 Inactive
@m-green
Copy link
Contributor

m-green commented Jun 13, 2019

Hi @NickColley - I think I've managed to use sections to combine this in a way that avoids duplication (and means that each type of user - HTML/Javascript - gets their guidance in one go). And it's much shorter! Let me know what you think.
I think some of the terms I've used might not be quite right - let me know if so.

CHANGELOG.md Outdated
@@ -268,72 +268,47 @@

([PR #1376](https://github.com/alphagov/govuk-frontend/pull/1376))

- Ensure GOV.UK Frontend component selectors cannot conflict when initialised
- Update and add the data-module attribute
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this title is too vague now

CHANGELOG.md Outdated

Components which have JavaScript functionality include a data attribute called `[data-module]`,
we have added the `govuk-` prefix as a namespace.
The `[data-module]` attribute in components now has a `gov-uk` namespace prefix. This is to avoid conflicts between attribute names, and make the attributes consistent with our style for class names.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gov-uk -> govuk-

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conflicts with what? Before it was clear what it was conflicting with I think.

CHANGELOG.md Outdated

This ensures the component's name does not conflict with other service level components
with the same name, which is consistent with our CSS class name convention.
The [button](https://design-system.service.gov.uk/components/button/#start-buttons) and [details](https://design-system.service.gov.uk/components/details/) components now also have the `[data-module]` attribute.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'now have' -> 'now use'?

CHANGELOG.md Outdated

If you're using Nunjucks macros, and are [using the `initAll` function](https://github.com/alphagov/govuk-frontend/blob/master/docs/installation/installing-with-npm.md#option-1-include-javascript) you will not be affected.
You'll need to update your CSS or JavaScript code, to make sure that JavaScript can find and initalise the components.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure CSS is relevant to this.

'JavaScript', 'your JavaScript'

CHANGELOG.md Outdated
</div>
```

If you are initialising components manually, you will need to add the `govuk-` prefix to any CSS selectors used to find GOV.UK Frontend components:
You'll also need to add `data-module="govuk-button"` to each:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect since we don't want to add 'govuk-button' to the details component

CHANGELOG.md Outdated
For example the accordion component selector has changed from `[data-module="accordion"]` to `[data-module="govuk-accordion"]`.
-- If you're using HTML and CSS

You'll need to add the `govuk-` prefix to the `[data-module]` attribute in each CSS selector.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplifying this to 'CSS' selector means this is no longer correct, as we want to help people using JavaScript not CSS

@NickColley
Copy link
Contributor Author

Spoke to Mark and we're going to try and simplify the CHANGELOG later in the release notes so we can unblock other work. We need to pair on this to get it right...

@NickColley NickColley merged commit 7d8d4a5 into v3 Jun 13, 2019
@NickColley NickColley deleted the namespace-data-modules branch June 13, 2019 12:20
peteryates added a commit to x-govuk/govuk-form-builder that referenced this pull request Aug 5, 2019
The GOV.UK Design System V3 has introduced a data module for buttons[0]
that allows JavaScript events to be registered. As per the documentation
it's now present by default

[0] alphagov/govuk-frontend#1443
peteryates added a commit to x-govuk/govuk-form-builder that referenced this pull request Aug 5, 2019
This change is required due to new approach to namespacing[0] in V3.0 of
the GOV.UK Design System

[0] alphagov/govuk-frontend#1443
peteryates added a commit to x-govuk/govuk-form-builder that referenced this pull request Aug 5, 2019
This change is required due to new approach to namespacing[0] in V3.0
of the GOV.UK Design System

[0] alphagov/govuk-frontend#1443
frankieroberto added a commit to govuk-ruby/govuk-design-system-rails that referenced this pull request Oct 30, 2019
This is for compatibility with [version 3.0.0](https://github.com/alphagov/govuk-frontend/releases/tag/v3.0.0) of the govuk-frontend, which addes a prefix to `data-module` attributes to avoid namespacing conflicts. See [issue 1443](alphagov/govuk-frontend#1443)
frankieroberto added a commit to govuk-ruby/govuk-design-system-rails that referenced this pull request Oct 30, 2019
This is for compatibility with [version 3.0.0](https://github.com/alphagov/govuk-frontend/releases/tag/v3.0.0) of the govuk-frontend, which addes a prefix to `data-module` attributes to avoid namespacing conflicts. See [issue 1443](alphagov/govuk-frontend#1443)
peteryates added a commit to x-govuk/govuk-form-builder that referenced this pull request Jan 9, 2020
The GOV.UK Design System V3 has introduced a data module for buttons[0]
that allows JavaScript events to be registered. As per the documentation
it's now present by default

[0] alphagov/govuk-frontend#1443
peteryates added a commit to x-govuk/govuk-form-builder that referenced this pull request Jan 9, 2020
This change is required due to new approach to namespacing[0] in V3.0 of
the GOV.UK Design System

[0] alphagov/govuk-frontend#1443
peteryates added a commit to x-govuk/govuk-form-builder that referenced this pull request Jan 9, 2020
This change is required due to new approach to namespacing[0] in V3.0
of the GOV.UK Design System

[0] alphagov/govuk-frontend#1443
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

5 participants